Using PolySharp to allow use of C#13 #132

DennisDyallo · 2024-07-30T17:31:27Z

Description

This update removes the need for the Yubico.DotnetPolyfills-project. This project was adding a couple of methods, such as the Range and the Index operator. Now, since adding Polysharp, this is no longer needed as Polysharp generates polyfills for newer C# language features and APIs at compile-time. This also allows us to use C# language version 13 instead of 7.3.

Examples of new syntax can be seen here in a stowed away branch in this repo.

Note
It's true that bringing in a dependency into any library should not be taken in lightly. This suggestion has been with the team for some time, and while we haven't committed to it yet, we want to explore the possibility of making such a change. We do believe measures can be taken to ensure stability and expected behavior. Rest assured, although the third party dependency in question appears to be well regarded, useful and stable, it won't be added without going through a due dilligence process.

To anyone else reading this, please feel free to add your comments or any alternatives to move the SDK forward in this regard.

Fixes: YESDK1316

Type of change

How has this been tested?

Unit tests pass and builds pass.

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have run dotnet format to format my code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

Add Polysharp and langversion 11

github-actions · 2024-07-30T18:17:08Z

Test Results: Windows

2 files 2 suites 4s ⏱️
3 777 tests 3 777 ✅ 0 💤 0 ❌
3 779 runs 3 779 ✅ 0 💤 0 ❌

Results for commit 8c4428c.

♻️ This comment has been updated with latest results.

github-actions · 2024-07-30T18:17:15Z

Test Results: Ubuntu

2 files 2 suites 5s ⏱️
3 769 tests 3 769 ✅ 0 💤 0 ❌
3 771 runs 3 771 ✅ 0 💤 0 ❌

Results for commit 8c4428c.

♻️ This comment has been updated with latest results.

github-actions · 2024-07-30T18:17:22Z

Test Results: MacOS

2 files 2 suites 5s ⏱️
3 769 tests 3 769 ✅ 0 💤 0 ❌
3 771 runs 3 771 ✅ 0 💤 0 ❌

Results for commit 8c4428c.

♻️ This comment has been updated with latest results.

iamcarbon · 2024-08-25T19:53:24Z

This introduces an untrusted third party dependency into a security critical library. It would be better to selectively port the individual Polyfills into core, then introduce a new dependency (and extend the chain of trust).

DennisDyallo · 2024-08-27T10:36:03Z

This introduces an untrusted third party dependency into a security critical library. It would be better to selectively port the individual Polyfills into core, then introduce a new dependency (and extend the chain of trust).

Hi @iamcarbon and thanks for your raised concern. It's true that bringing in a dependency into any library should not be taken in lightly. This suggestion has been with the team for some time, and while we haven't committed to it yet, we want to explore the possibility of making such a change. We do believe measures can be taken to ensure stability and expected behavior. Rest assured, although the third party dependency in question appears to be well regarded, useful and stable, it won't be added without going through a due dilligence process.

To you and anyone else reading this, please feel free to add your comments or any alternatives to move the SDK forward in this regard.

@dainnilsson @GregDomzalski FYI

Update target framework to .NET 9.0 Updated the target framework from .NET 6.0 to .NET 9.0. Added a commented-out section in the project file with instructions for testing against local changes to the YubiKey library, including a `ProjectReference` to a local YubiKey project file.

CryptographicOperations class duplicated in both Core and Yubikey

…c and remove duplicate implementation

…rtificate loading method

saved in different branch: https://github.com/Yubico/Yubico.NET.SDK/blob/dennisdyallo/advanced-syntax/Yubico.YubiKey/src/Yubico/YubiKey/CSharpLangFeaturesExamples.cs

…d remove old implementation

…mNumberGeneratorExt

DennisDyallo · 2025-02-13T08:35:53Z

Yubico.YubiKey/tests/integration/Yubico/YubiKey/Piv/Commands/AuthMgmtKeyCmdTests.cs

@@ -88,7 +88,7 @@ public void AuthKey_Aes_Succeeds()
                    0x51, 0x52, 0x53, 0x54, 0x55, 0x56, 0x57, 0x58
                };

-                var initCmd = new InitializeAuthenticateManagementKeyCommand(true, PivAlgorithm.TripleDes);
+                var initCmd = new InitializeAuthenticateManagementKeyCommand(true, PivAlgorithm.TripleDes); // TODO this test only works with 5.4.3 and lower


Will handle this in test rework case

Yubico.YubiKey/tests/integration/Yubico/YubiKey/Piv/Commands/AuthMgmtKeyCmdTests.cs

…nfig)

… for compatibility

…project

…ation for project structure

…nd SmartCardDeviceListenerTests

…ject files" This reverts commit 85b35f0.

github-actions · 2025-02-13T10:16:55Z

Package	Line Rate	Branch Rate	Complexity	Health
Yubico.Core	40%	31%	4311	➖
Yubico.YubiKey	50%	46%	19632	➖
Summary	48% (33138 / 69216)	44% (8226 / 18903)	23943	➖

Minimum allowed line rate is 40%

DennisDyallo added 7 commits July 30, 2024 18:37

Remove PolyFills project.

2b71bc2

Add Polysharp and langversion 11

Fix warnings

fffe147

Analysis warning

d338850

C#12 Language refactors

e219573

Disable analysis on SampleProjects

f51b5f1

Merge branch 'develop' into dennisdyallo/polysharp

9729977

Ignore warning on tests

687083e

DennisDyallo changed the title ~~Replaceing Yubico.DotnetPolyFills with PolySharp~~ Replacing Yubico.DotnetPolyFills with PolySharp Jul 30, 2024

Add .net6 to github setup for test project

8a8d09f

Disable analysis on test-app

85a888e

DennisDyallo changed the title ~~Replacing Yubico.DotnetPolyFills with PolySharp~~ Replacing Yubico.DotnetPolyFills with PolySharp. Bumping LanguageVersion to C#12. Jul 30, 2024

DennisDyallo added 10 commits January 11, 2025 17:24

fix merge conflicts

9dd6ff8

Updated PolySharp 1.15. LangVersion 13

4d1d89b

Visual Studio updated sln file

e14e62d

bump to .net9 in setup dotnet gh actions and globaljson

2bf20a7

misc: fix warnings

9349b6f

misc: fix warnings

a5ea782

misc: fix warnings

f3d15f1

Bump to .NET9 in sample and test projects

4172c71

tests: fix warnings

7a7a3ff

DennisDyallo changed the title ~~Replacing Yubico.DotnetPolyFills with PolySharp. Bumping LanguageVersion to C#12.~~ Using PolySharp to allow use of C#13 Jan 13, 2025

Merge branch 'develop' into dennisdyallo/polysharp

cf10d92

DennisDyallo marked this pull request as ready for review February 6, 2025 10:09

DennisDyallo requested a review from dainnilsson February 6, 2025 10:09

DennisDyallo added 6 commits February 6, 2025 12:35

Merge branch 'develop' into dennisdyallo/polysharp

194397a

refactor: remove obsolete Index and Range implementations

082769c

refactor: use internal static classes CryptographicOperations

7961f8b

CryptographicOperations class duplicated in both Core and Yubikey

refactor: change CryptographicOperations class from internal to publi…

e5cd2e7

…c and remove duplicate implementation

misc: remove unused project DotnetPolyfills

3dd3c82

tests: add Microsoft.Bcl.Cryptography package reference and update ce…

4b413c4

…rtificate loading method

DennisDyallo force-pushed the dennisdyallo/polysharp branch from f41b4b1 to 4b413c4 Compare February 13, 2025 06:27

DennisDyallo added 9 commits February 13, 2025 07:30

Merge branch 'develop' into dennisdyallo/polysharp

eb37476

delete: remove CSharpLangFeaturesExamples class and related code

e949aa4

saved in different branch: https://github.com/Yubico/Yubico.NET.SDK/blob/dennisdyallo/advanced-syntax/Yubico.YubiKey/src/Yubico/YubiKey/CSharpLangFeaturesExamples.cs

docs: update README structure and improve section headings for clarity

b2eb647

misc: run dotnet format

5e4628d

misc: specify type for readerStateList in CheckForUpdates method

3e46970

misc: move CryptographicOperations to System.Security.Cryptography an…

01fdf50

…d remove old implementation

misc: update license references for CryptographicOperations and Rando…

447c03f

…mNumberGeneratorExt

misc: use var for readerStateList in CheckForUpdates method

aafd2fe

misc: inadvertent change regarding reverted (SdkPlatformInfo)

5638712

DennisDyallo commented Feb 13, 2025

View reviewed changes

Yubico.YubiKey/tests/integration/Yubico/YubiKey/Piv/Commands/AuthMgmtKeyCmdTests.cs Show resolved Hide resolved

DennisDyallo added 11 commits February 13, 2025 09:37

misc: enable implicit object creation when type is apparent (editorco…

020cef2

…nfig)

misc: fix namespaces and minor things

97a5c8a

misc: prevent automatic updates for PolySharp package

7e4d0fa

misc: clean up code formatting and suppress string comparison warning…

2d1ae97

… for compatibility

misc: temporary disable code style warning

2b8a74e

misc: add appsettings.json for unit tests configuration

25e1ceb

misc: reorder PackageReference for Xunit.SkippableFact in unit tests …

18acd83

…project

misc: remove references to Yubico.DotNetPolyfills and update document…

a49ec72

…ation for project structure

misc: downgrade target framework from net9.0 to net8.0 in project files

85b35f0

misc: comment out listener creation tests in HidDeviceListenerTests a…

9563c41

…nd SmartCardDeviceListenerTests

Revert "misc: downgrade target framework from net9.0 to net8.0 in pro…

8c4428c

…ject files" This reverts commit 85b35f0.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Using PolySharp to allow use of C#13 #132

Using PolySharp to allow use of C#13 #132

DennisDyallo commented Jul 30, 2024 •

edited

Loading

github-actions bot commented Jul 30, 2024 •

edited

Loading

github-actions bot commented Jul 30, 2024 •

edited

Loading

github-actions bot commented Jul 30, 2024 •

edited

Loading

iamcarbon commented Aug 25, 2024

DennisDyallo commented Aug 27, 2024 •

edited

Loading

DennisDyallo Feb 13, 2025

github-actions bot commented Feb 13, 2025

Using PolySharp to allow use of C#13 #132

Are you sure you want to change the base?

Using PolySharp to allow use of C#13 #132

Conversation

DennisDyallo commented Jul 30, 2024 • edited Loading

Description

Type of change

How has this been tested?

Checklist:

github-actions bot commented Jul 30, 2024 • edited Loading

Test Results: Windows

github-actions bot commented Jul 30, 2024 • edited Loading

Test Results: Ubuntu

github-actions bot commented Jul 30, 2024 • edited Loading

Test Results: MacOS

iamcarbon commented Aug 25, 2024

DennisDyallo commented Aug 27, 2024 • edited Loading

DennisDyallo Feb 13, 2025

Choose a reason for hiding this comment

github-actions bot commented Feb 13, 2025

DennisDyallo commented Jul 30, 2024 •

edited

Loading

github-actions bot commented Jul 30, 2024 •

edited

Loading

github-actions bot commented Jul 30, 2024 •

edited

Loading

github-actions bot commented Jul 30, 2024 •

edited

Loading

DennisDyallo commented Aug 27, 2024 •

edited

Loading